agent: @U0AJM7X8FBR Supabase - prevent API keys in Org Accounts.#13
agent: @U0AJM7X8FBR Supabase - prevent API keys in Org Accounts.#13sweetmantech wants to merge 1 commit intomainfrom
Conversation
|
Updates to Preview Branch (agent/-u0ajm7x8fbr-supabase---preven-1773937671525) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
📝 WalkthroughWalkthroughA PostgreSQL migration adds a trigger function to the Changes
Sequence DiagramsequenceDiagram
actor Client
participant API as Insert/Update Request
participant Trigger as prevent_org_account_api_keys()
participant OrgTable as account_organization_ids
participant APIKeysTable as account_api_keys
Client->>API: Insert/Update API Key
API->>Trigger: BEFORE INSERT OR UPDATE fires
Trigger->>OrgTable: Query for organization_id = NEW.account
OrgTable-->>Trigger: Check result
alt Organization Account Found
Trigger-->>API: RAISE EXCEPTION
API-->>Client: Operation rejected
else Non-Organization Account
Trigger-->>APIKeysTable: Return NEW (allow operation)
APIKeysTable-->>Client: Insert/Update succeeds
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
supabase/migrations/20260319000000_prevent_org_account_api_keys.sql (2)
1-4: Add a follow-up data remediation for existing violating keys.This migration blocks future writes but does not revoke/delete already-issued org-account API keys created before this fix. Consider a companion migration/job to audit and remediate existing violations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260319000000_prevent_org_account_api_keys.sql` around lines 1 - 4, Add a follow-up data remediation that finds and revokes/deletes existing API keys already issued to organization accounts (those whose account_id appears in account_organization_ids.organization_id) — write a new migration or one-off job that queries the api_keys table for keys where api_keys.account_id IN (SELECT organization_id FROM account_organization_ids), then for each matching row perform the approved remediation (e.g., set api_keys.revoked_at = now() or delete rows), record an audit/log entry, and ensure you run it in a transaction with a pre-check/backup step so you can revert if needed; reference the account_organization_ids table and the api_keys rows in your implementation.
23-25: Scope UPDATE trigger to account changes only.The trigger currently uses
BEFORE INSERT OR UPDATE, causing it to re-evaluate the organization check on any update—including changes toname,key_hash, orlast_usedcolumns. Since the function only checksNEW.accountagainstaccount_organization_ids, scoping toUPDATE OF accountis more efficient and prevents unnecessary evaluations:Proposed change
CREATE TRIGGER prevent_org_account_api_keys_trigger - BEFORE INSERT OR UPDATE ON public.account_api_keys + BEFORE INSERT OR UPDATE OF account ON public.account_api_keys FOR EACH ROW EXECUTE FUNCTION public.prevent_org_account_api_keys();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260319000000_prevent_org_account_api_keys.sql` around lines 23 - 25, The trigger prevent_org_account_api_keys_trigger on public.account_api_keys is firing for any UPDATE, but the function only validates NEW.account; change the trigger definition so it fires BEFORE INSERT OR UPDATE OF account (i.e., scope the UPDATE to the account column only) to avoid unnecessary evaluations on unrelated column changes like name, key_hash, or last_used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260319000000_prevent_org_account_api_keys.sql`:
- Around line 6-21: The trigger function prevent_org_account_api_keys currently
runs with default SECURITY INVOKER and will see zero rows when
account_organization_ids has RLS without policies; to fix, either convert the
function to SECURITY DEFINER and set an explicit search_path (e.g., add
"SECURITY DEFINER SET search_path = pg_catalog, public") and ensure the definer
role has SELECT rights on account_organization_ids, or keep SECURITY INVOKER but
add an RLS policy on account_organization_ids that allows SELECT for the
application roles executing the trigger; update the function declaration or RLS
policy accordingly and verify the definer/role permissions so the SELECT in
prevent_org_account_api_keys reliably sees rows.
---
Nitpick comments:
In `@supabase/migrations/20260319000000_prevent_org_account_api_keys.sql`:
- Around line 1-4: Add a follow-up data remediation that finds and
revokes/deletes existing API keys already issued to organization accounts (those
whose account_id appears in account_organization_ids.organization_id) — write a
new migration or one-off job that queries the api_keys table for keys where
api_keys.account_id IN (SELECT organization_id FROM account_organization_ids),
then for each matching row perform the approved remediation (e.g., set
api_keys.revoked_at = now() or delete rows), record an audit/log entry, and
ensure you run it in a transaction with a pre-check/backup step so you can
revert if needed; reference the account_organization_ids table and the api_keys
rows in your implementation.
- Around line 23-25: The trigger prevent_org_account_api_keys_trigger on
public.account_api_keys is firing for any UPDATE, but the function only
validates NEW.account; change the trigger definition so it fires BEFORE INSERT
OR UPDATE OF account (i.e., scope the UPDATE to the account column only) to
avoid unnecessary evaluations on unrelated column changes like name, key_hash,
or last_used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cdcd6c5c-49bd-4504-8edd-cd5cd0e9af60
📒 Files selected for processing (1)
supabase/migrations/20260319000000_prevent_org_account_api_keys.sql
| CREATE OR REPLACE FUNCTION public.prevent_org_account_api_keys() | ||
| RETURNS trigger AS $$ | ||
| BEGIN | ||
| IF EXISTS ( | ||
| SELECT 1 | ||
| FROM public.account_organization_ids | ||
| WHERE organization_id = NEW.account | ||
| ) THEN | ||
| RAISE EXCEPTION | ||
| 'Cannot create an API key for an organization account (account_id: %)', | ||
| NEW.account; | ||
| END IF; | ||
|
|
||
| RETURN NEW; | ||
| END; | ||
| $$ LANGUAGE plpgsql; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== RLS/policies/grants touching account_organization_ids =="
rg -n -C3 'account_organization_ids|ENABLE ROW LEVEL SECURITY|CREATE POLICY|GRANT .*account_organization_ids' supabase/migrations
echo
echo "== Function security/search_path declarations =="
rg -n -C3 'prevent_org_account_api_keys|SECURITY DEFINER|search_path' supabase/migrationsRepository: recoupable/database
Length of output: 25027
🏁 Script executed:
cat -n supabase/migrations/20260319000000_prevent_org_account_api_keys.sqlRepository: recoupable/database
Length of output: 1096
🏁 Script executed:
# Search for any CREATE POLICY statements on account_organization_ids more explicitly
rg -n 'CREATE POLICY.*account_organization_ids|CREATE POLICY' supabase/migrations/20251204000000_create_account_organization_ids.sqlRepository: recoupable/database
Length of output: 45
🏁 Script executed:
# Check if there are RLS policies defined elsewhere (policies might be in separate migration)
rg -n 'account_organization_ids.*CREATE POLICY|CREATE POLICY.*account_organization_ids|ON.*account_organization_ids' supabase/migrationsRepository: recoupable/database
Length of output: 2180
🏁 Script executed:
# Check for GRANT statements on account_organization_ids
rg -n 'GRANT.*account_organization_ids|GRANT.*SELECT.*account_organization_ids' supabase/migrationsRepository: recoupable/database
Length of output: 45
🏁 Script executed:
# Check if account_organization_ids has any RLS policies defined anywhere
rg -n 'account_organization_ids' supabase/migrations | grep -i 'policy\|grant\|enabled'Repository: recoupable/database
Length of output: 45
🏁 Script executed:
# Look for the entire content of the account_organization_ids creation migration
cat -n supabase/migrations/20251204000000_create_account_organization_ids.sql | tail -20Repository: recoupable/database
Length of output: 1134
🏁 Script executed:
# Check if there are any RLS-related files or separate policy migrations
fd -t f 'policy\|rls' supabase/migrations/Repository: recoupable/database
Length of output: 45
🏁 Script executed:
# Search for all CREATE POLICY statements across entire migrations directory
rg -n '^CREATE POLICY' supabase/migrations/Repository: recoupable/database
Length of output: 435
Function will silently fail to enforce the check for non-superuser roles due to missing RLS policy coverage.
The account_organization_ids table has RLS enabled but no policies defined. Combined with SECURITY INVOKER (the default), the function's SELECT query will return zero rows when executed by an application-level authenticated role, causing the validation to always pass silently.
To fix: Either (1) add SECURITY DEFINER SET search_path = pg_catalog, public to the function and ensure the triggering role has necessary grants, or (2) define an explicit RLS policy on account_organization_ids that permits SELECT access to the roles that will execute this trigger.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260319000000_prevent_org_account_api_keys.sql` around
lines 6 - 21, The trigger function prevent_org_account_api_keys currently runs
with default SECURITY INVOKER and will see zero rows when
account_organization_ids has RLS without policies; to fix, either convert the
function to SECURITY DEFINER and set an explicit search_path (e.g., add
"SECURITY DEFINER SET search_path = pg_catalog, public") and ensure the definer
role has SELECT rights on account_organization_ids, or keep SECURITY INVOKER but
add an RLS policy on account_organization_ids that allows SELECT for the
application roles executing the trigger; update the function declaration or RLS
policy accordingly and verify the definer/role permissions so the SELECT in
prevent_org_account_api_keys reliably sees rows.
1 participant
Automated PR from coding agent.
Prompt: @U0AJM7X8FBR Supabase - prevent API keys in Org Accounts.
• Actual: It's possible to create an API key for an accountID that exists in the organization_id column of the account_organization_ids table.
• Required: new migration which prevents API keys being created with account column equal to an accountID which exists in the organization_id column of the account_organization_ids table.
Summary by CodeRabbit